feat(registry): record arctic-1's deployed contracts - #76
Conversation
Three contracts deployed to arctic-1 from a local run, so a deployment binds them instead of deploying its own on every restart. defi-amm 0x225af59603bb554686adfbb2869af4cec12488a1 tokenops-erc20 0xe66344c8ed6dbde610725cd7e3359b1fe4d7ff26 tokenops-erc721 0x815299db5f8e3c6c42356655429cf2701e502bea The run recorded 3 deployed and 0 copied unverified, so nothing here is carried through from a file this run did not check. Verified independently of the run that wrote it. Every address holds code, the recorded hash is keccak256 of the code the chain serves, and that code is byte-identical to what the pinned compiler produces locally. So these are our contracts, not merely addresses that hold something. Verified end to end: a binary carrying this file bound all three and sent no deployment, and the transactions it then sent moved the contracts' state. arctic-1's AMM reserve is no longer at its floor and its ERC721 token 1 has an owner, so the transactions executed rather than reverting. The tripwire test asserting an empty compiled-in registry said a chain file needs its own test naming it. It has one. Three mutations, three caught: a changed address, a changed genesis hash, a dropped contract. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PR SummaryMedium Risk Overview Separately, the StorageRW scenario adds fixed Reviewed by Cursor Bugbot for commit b0e54d4. Bugbot is set up for automated code reviews on this repo. Configure here. |
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
Ships the first compiled-in chain file (arctic-1) plus a test naming its contents; the JSON parses and validates cleanly and the test pins the chain id, genesis hash, a negative genesis-hash match, the contract count, and the addresses. No blockers — the remaining notes are a vacuous code-hash assertion and documentation/profile text that the new file makes stale.
Findings: 0 blocking | 3 non-blocking | 1 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- [suggestion] Several comments and docs now state the opposite of what the binary ships:
registry/chains/README.md:7("This directory is empty of chain files today. No contracts exist on arctic-1, atlantic-2 or pacific-1 yet"),registry/registry.go:18-20("The directory ships empty"),registry/chains_test.go:16(same claim, and itsfound == 0log branch is now dead), andgenerator/registry_test.go:214("The compiled-in registry ships empty"). Worth updating in this PR so the first committed chain file doesn't leave four stale claims behind. - [suggestion]
profiles/arctic-1.jsonstill nameschainId: 713715with nogenesisHash, so it cannot bind the addresses this PR records. Today it only runsEVMTransfer(no contract bindings, so no resolve happens), but adding any contract scenario to it will now fail at startup via theHasChainIDguard ingenerator/prepare.go:250— "the registry describes chain 713715 but this run names no genesisHash". Adding"genesisHash": "8ef5b0c0…bf035"to the profile now makes the recorded contracts usable and avoids a confusing first failure. - 1 suggestion(s)/nit(s) flagged inline on specific lines.
| if got := strings.ToLower(contract.Address.Hex()); got != address { | ||
| t.Errorf("%s is recorded at %s, want %s", name, got, address) | ||
| } | ||
| if contract.CodeHash == (common.Hash{}) { |
There was a problem hiding this comment.
[suggestion] This assertion can never fire: Chain.validate already rejects a zero codeHash (registry/registry.go:185), so any chain file that reached Load without erroring has a non-zero hash on every contract. The test therefore pins the addresses but leaves the code hashes entirely unpinned — the one field whose corruption makes every cell reading this image fail Verify at startup can be edited without any test changing, which is the drift this test's doc comment says it exists to prevent.
Pin the exact hashes alongside the addresses, e.g. widen want to a struct:
want := map[string]struct{ address, codeHash string }{
"defi-amm": {
"0x225af59603bb554686adfbb2869af4cec12488a1",
"0xd3b745d66f41b203732768f63c3f58a08be56a4c007d4ed88686d5230d7d54cd",
},
// ...
}and compare strings.ToLower(contract.CodeHash.Hex()) against it.
Review found that StorageRW's doc claimed the margin absorbs the gap between its priced calls and read's expensive shape. Settings.Validate accepts a GasMargin of 1, and at 1 nothing absorbs it: the first read of a written slot would land in a block having burned its whole limit, which is the failure this sizing exists to remove. read costs most against a slot that already holds a value, and every call the scenario prices reads an untouched one, so no measurement reaches that shape. What the write and rmw models miss is a cold slot read, which EIP-2929 prices at 2,100 and which is not one of the costs Sei moves. The scenario adds twice that as a constant, so correctness no longer depends on how an operator sets a knob. Two mutations, two caught: the headroom removed, and the headroom shrunk below a cold read. Also moved gasProbeSlot out of CreateContractTransaction's doc comment. Inserting it there left the function undocumented and turned a warning about that function's PRNG draw order into commentary on a package-level big.Int. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Collapsed into #73. Reviewing these apart cost more than the separate boundaries were worth: the Disperse fix and the Value field were in #74, the read headroom in #76, and the newest estimator fixes in #73, so no branch was individually correct and each review pass had to reason about where a fix actually lived. Every commit is preserved in #73's history. Nothing is dropped. |
…#73) Replaces the numbers this tool hard-codes with numbers it asks the chain for. Sei charges 72,000 for a storage write that takes a slot from zero, against the EVM default of 20,000. Every gas constant in the repo was calibrated against the default, so every contract scenario was short on Sei — and a short limit does not fail visibly. The transaction reaches a block, burns the whole limit, and a run with `trackReceipts` off reports it as sent. | | was | arctic-1 needs | |---|---|---| | `AMM.swapAToB` | 85,000 | 185,711 | | `ERC20.transfer` | 72,156 | 175,242 | | `ERC721.mint` | 22,460 | 175,000 | | fee cap (all paths) | 20 gwei | 55 gwei | ## What it does A scenario declares `GasEstimateCalls()`; the run prices them once at startup, after the contracts are bound, and the send path reads a number. `ContractScenarioBase` does not implement the method, so a scenario added without one does not compile. The fee cap comes from `eth_gasPrice` at startup. Contract addresses for arctic-1 ship in `registry/chains/`, so a run binds them instead of redeploying on every restart. Pricing failure stops the run. There is no fallback constant, because a fallback constant is what this removes. ## Also here - `/healthz` and `/readyz`, and readiness now drops on every exit rather than only on a signal - an `amm` scenario, and ERC721's gas constant fixed before it was deleted ## Verification Run against arctic-1: contracts bound from the registry with no deployment sent, gas quoted per operation, transactions executed — the AMM reserve moved off its floor and ERC721 token 1 has an owner. Every fix broken on purpose first. Four review rounds found five, three, two and one defect; all are fixed here. Twice a fix introduced its own mirror image: the value-carrying estimate now funds its caller through a state override, and the per-quote timeout runs through `WithinBudget` so a hung endpoint reads as a failure rather than a clean shutdown. `gofmt`, `go vet`, `golangci-lint` clean; 15 packages passing. ## Known - committing `registry/chains/arctic-1.json` makes `genesisHash` required for any arctic-1 profile that drives a contract scenario; `profiles/arctic-1.json` now carries it, but a profile kept outside this repo needs it added - the gas tip is still a constant while the cap is chain-derived, and StorageRW packs its calldata twice per send; both are flagged non-blocking on the PR - ERC721's token counter restarts at 1, so reusing a recorded contract wastes gas until it passes the previous high-water mark (PLT-1107) - gas and fee cap are resolved once and held; a long run outliving either is undetected - `--duration N` exits 1 at the end of a bounded run Collapsed from #74, #75 and #76, which are closed. Deploys via sei-protocol/platform#1587. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Sixth on the stack. The first chain file this repo ships.
A deployment now binds these instead of deploying its own on every restart.
How they were produced
A local run, as
registry/chains/README.mdprescribes — not from a deployed pod,which could not produce a file the image then contains. The binary was built from
this branch, so the recorded code hashes match the bindings production runs.
The run reported 3 deployed, 0 copied unverified, so nothing here is carried
through from a file it did not check itself.
Verified three ways, none of them trusting the run that wrote it
The code is ours. Every address holds code; the recorded hash is keccak256 of
what the chain serves; and that code is byte-identical to what the pinned
compiler produces locally.
The entry binds. A binary carrying this file, run against arctic-1:
The transactions execute.
trackReceiptswas off, so "sent" would not haveproved anything. On-chain state after the run:
The reserve moved off its floor and token 1 has an owner, so the swaps and mints
ran rather than reverting.
It also validated the gas work against a real chain
This is the first run of #73 and #74 against arctic-1 rather than a state
override. The quotes match what I measured by injection, within a few hundred
gas:
And the fee cap resolved from the chain:
gas price 11000000000 wei, fee cap 55000000000 wei (x5.0)— where the constant this stack removed declared 20 gwei.The test
TestLoadWithNoPathsReadsOnlyTheBinaryasserted the compiled-in registry wasempty, and its failure message said what to do about it:
It has one. It pins the chain id, the genesis hash, all three addresses, and that
each entry carries a code hash for
Verifyto check. It also asserts a wronggenesis hash does not match, because keying on the chain id alone is the failure
this file's whole design exists to prevent.
Three mutations, three caught: a changed address, a changed genesis hash, a
dropped contract.
Known, tracked
PLT-1107: ERC721's token counter restarts at 1 per process, so a second run
against this recorded contract mints into already-owned ids until it passes the
first run's high-water mark. That is throughput waste, not a failure — #73 prices
the scenario against a deliberately unminted id, so the limit still covers the
expensive shape.
PLT-1106: this bootstrap was a hand-shaped profile around a step that is not the
point. A deploy-only entrypoint would remove the four constraints an operator has
to get right at once.
🤖 Generated with Claude Code